REST API: Themes: Expose some additional basic fields #222
Conversation
|
/cc @kadamwhite |
|
Looks great! Are you interested in adding tests for this? |
Thanks! Sure, I'll add some tomorrow 😄 |
|
Update, I've fixed the failing unit tests and lint. I haven't yet added new tests, since I'm still struggling with setting up the tests environment on my Linux box. I asked on Core Slack: https://wordpress.slack.com/archives/C02RQBWTW/p1586985925240400 |
ockham
changed the title
|
I managed to get unit tests to run locally, and added a few more fields and corresponding tests (see updated PR description). I'd say this is ready for review 🙂 |
Display the current theme name in the template selector of `edit-site`. Since the `/themes` endpoint currently only exposes the `theme_supports` field for each theme, this PR adds a number of additional fields to it, using the `rest_prepare_theme` hook. (I also filed a WordPress/wordpress-develop#222 against Core to extend the, err, upstream endpoint accordingly so that we can eventually drop the filter.) In addition, this PR adds a new `getCurrentTheme` selector and related reducer/resolver/action to `core-data` (including a `themes` reducer for better normalization). Finally, that selector is used to render the theme name in `edit-site`'s template selector. The fields I've chosen to add should be sufficient to implement the on-hover previews seen at #20469 (comment). However, this PR doesn't implement those previews yet, since we need to implement the underlying fly-out menu component first (see #20470).
|
cc/ @mcsf for review. Because it's a themes endpoint! 🎉 |
Good times! Looks great, as far as I'm concerned. I'd defer to Timothy or K Adam for the final green light, though. |
@kadamwhite @TimothyBJacobs Pretty pleaaase? 😊 |
There was a problem hiding this comment.
Thanks for working on this @ockham! Left comments with a couple of minor changes to the formatting of the response.
src/wp-includes/rest-api/endpoints/class-wp-rest-themes-controller.php
Outdated
Show resolved
Hide resolved
src/wp-includes/rest-api/endpoints/class-wp-rest-themes-controller.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
|
Hey @ockham just checking in on this! |
Thanks! I'm back today from 2 weeks of maintenance rotation, picking this up again 👍 |
|
@ockham To clarify, the type should still be string but have a format => uri. |
|
Merged in r47921. |
In #21578, I added a few fields to Core's `/themes` endpoint, for use by the Site Editor's Template Switcher (see both #21578 and #21768). I then submitted those changes as a [PR](WordPress/wordpress-develop#222) against Core. That PR underwent a number of modifications and was eventually merged; the new fields will be part of the `/themes` endpoint exposed by Core starting from the next WP release. This PR updates the fields added by Gutenberg to follow the same semantics, as well as the callsites that use that endpoint.
The
/wp/v2/themesendpoint currently only exposes each theme's theme_supports field, but nothing else -- not even theme name or title.See https://core.trac.wordpress.org/ticket/49906.
Needed for WordPress/gutenberg#21578 -- you can use that PR to test this one 🙂 I'm adding all the fields that I think will be needed to implement WordPress/gutenberg#20469 (comment):
That is:
authorauthor_nameauthor_uridescriptionnamescreenshotstylesheettemplatetheme_uriversionImplementation loosely inspired by https://github.com/Automattic/jetpack/blob/f6f313d268c336491c8bca1afcf3e5d3663597ce/json-endpoints/jetpack/class.jetpack-json-api-themes-endpoint.php.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.